Add support for C# 14 user-defined compound assignment operators - #3972
Add support for C# 14 user-defined compound assignment operators#3972siegfriedpammer wants to merge 3 commits into
Conversation
christophwille
left a comment
There was a problem hiding this comment.
Automated high-effort review (multi-agent, each finding independently verified against the PR head). Overall: the PR handles the straightforward Roslyn lvalue cases well, but the new pattern matchers are loose at several boundaries. The most severe defects break recompilation of plain C# 14 code (explicit interface operator implementations, virtual operators called through a derived-typed receiver), and the static-operator compound-assign path can now silently change runtime semantics when a type declares both static and instance operators. Secondary issues are settings-gating gaps (CheckedOperators, UnsignedRightShift) and the signature-blind checked-equivalent probe inherited by the new paths.
10 findings posted as inline comments, ordered by severity there. Summary:
- Explicit interface implementations of instance compound operators decompile as plain methods (OperatorDeclaration.cs) - output fails to compile (CS0539).
- Derived-typed receivers get a cast that becomes the assignment target (CallBuilder.cs) - emits
(C)d += n(CS0131). - Non-lvalue receivers become an invalid assignment LHS (ReplaceMethodCallsWithOperators.cs) -
GetC() += n;(CS0131). - Type-parameter cast stripped without checking constraints (ReplaceMethodCallsWithOperators.cs) - non-compiling or wrong-binding output.
x op= yprinted for static operator calls even when an instance compound operator exists (TransformAssignment.cs) - recompiled code binds to the instance operator, silently different runtime behavior.- Static / value-returning
op_*Assignment(F#, C++/CLI) now rendered as operator declarations (TypeSystemAstBuilder.cs) - invalid C#. - No void-return check on the instance-operator rewrite (ReplaceMethodCallsWithOperators.cs).
- Checked compound-assignment names not gated on
settings.CheckedOperators(ReplaceMethodCallsWithOperators.cs). HasCheckedEquivalentis signature-blind (ReplaceMethodCallsWithOperators.cs) - spuriousuncheckedwrappers.op_UnsignedRightShiftAssignmentnot gated onsettings.UnsignedRightShift(ReplaceMethodCallsWithOperators.cs).
46beab7 to
d218467
Compare
a361886 to
644dfea
Compare
C# 14 lets a type declare instance compound assignment operators (operator +=, operator ++, and their checked forms), which the compiler emits as void-returning op_*Assignment methods. Classify those methods as operators in the type system - by name and required shape (instance, void, correct arity, no ref/params) - and gate it on a new decompiler setting and a matching TypeSystemOptions flag, so a lower language version keeps them as plain [SpecialName] methods. Model the new operator declarations and their metadata names, and give the resolver the two-phase binding rules "x op= y" follows: instance operators reachable from the static type of x, with the static operators considered only when none applies. This is the type-system foundation the rest of the feature builds on. Assisted-by: Claude:claude-fable-5:Claude Code Assisted-by: Claude:claude-opus-4-8:Claude Code
Fold a call to an instance compound assignment operator, X::op_AdditionAssignment(x, y), back into x += y (and x++, the checked forms), rewriting at the AST level in ReplaceMethodCallsWithOperators rather than introducing a new IL instruction. The form takes its operator from the static type of x and needs x to stay an assignable variable, so the receiver is protected end to end: the reader materializes a reference-type receiver into a stack slot, and inlining, copy propagation, foreach and using all refuse to replace it with something that is not an assignable variable or that would bind a different operator - redirecting to a copy where the variable would otherwise become read-only, so foreach and using statements are still emitted. Includes the round-trip, pretty, IL-pretty and ugly test fixtures. Assisted-by: Claude:claude-opus-4-8:Claude Code
Show an instance compound assignment operator as "operator +=(int) : void" in the tree and tooltips instead of its op_*Assignment metadata name, and list the C# 14 and 15 language versions in ilspycmd's -lv help. Assisted-by: Claude:claude-opus-4-8:Claude Code
644dfea to
aff4d4a
Compare
christophwille
left a comment
There was a problem hiding this comment.
Review: C# 14 user-defined compound assignment operators
Overall the feature is solid: the two-phase binding model, the instance/static shadowing guard and the receiver-lvalue protection are well thought out and well covered by fixtures. The problems below all sit at the seams of that guard. I went through the three commits (recognize / decompile / render, 40 files) with several independent passes; one candidate (the HasDefaultStackSlotType change) was refuted empirically (no output diff master vs PR) and is not listed.
Correctness (inline comments carry the details)
in-parameter operators defeat the shadow/rebind check -HandleCompoundAssign(TransformAssignment.cs:403) andWouldRebindOperator(IMethod, IType, ICompilation)(CSharpResolver.cs:1369) feed aByReferenceTypeinto overload resolution, which is applicable to nothing, so the check is always "not shadowed".x = x + ywithstatic operator +(Foo, in Foo)next tooperator +=(in Foo)folds tox += y, which C# 14 binds to the instance operator.UnwrapByRef()(as CallBuilder.cs:1752 already does) fixes both. Noin-parameter operator exists in the fixtures.- Statement-level
x++/++s/ foreach-local receiver escape the guard on three paths: the dead-store branch ofTransformPostIncDecOperator(and...WithInlineStore),FixRemainingIncrementswhen the store variable is still an object-typed stack slot, andCanBeDeconstructedInForeach(the deconstruction branch runs before the stloc branch that has the guard). - Receiver materialization for 0-parameter operators (
op_IncrementAssignmentetc.) stores the receiver slot beforeFlushExpressionStack()runs, because the flush sits inside the per-parameter loop. Reproduced with hand-assembled IL:Foo(A(), ++x)whereA()reassignsxdecompiles to increment the oldx. Roslyn happens to emitdupfor this shape, so C#-compiled input is unaffected; other compilers/weavers are not. - Receiver machinery keys on
IsOperator && !IsStatic, not on the compound-assignment shape (ILReader.cs:1842,UserDefinedCompoundAssign.IsCompoundAssignmentReceiverUse), so C++/CLI-style value-returning instance operators get forced into stack slots that inlining then refuses to fold (R r = GetR(); r.op_Addition(x);), and with the setting off such calls no longer reach any operator branch inCallBuilder(->AmbiguousMatch-> casts).
Cleanup
IsShadowedByInstanceOperatoris not gated by the setting at its four callers and walks the type hierarchy twice per call (plain + checked name) plus an O(n^2) dedup; everyx = x + y/++xondecimal/DateTime/BigInteger/... pays for it even when the feature is off.CopyPropagation.CannotReplaceCompoundAssignmentReceiverre-inlines the match thatUserDefinedCompoundAssign.IsCompoundAssignmentReceiverUse(added by this PR, used by ILInlining/UsingTransform) already expresses.MetadataMethod.IsUserDefinedCompoundAssignmentOperatorcarries two stacked<summary>blocks; the first belongs onIsCompoundAssignmentOperatorSignature, which has none.OperatorDeclaration.IsCompoundAssignmentrelies on enum order (type >= AdditionAssignment).CSharpResolver.PruneCandidatesHiddenByDerivedApplicable/IsApplicableduplicateOverloadResolution.AddMethodLists;ReplaceMethodCallsWithOperators.IsValidAssignmentTarget/IsAssignableTargetoverlap each other and restateILInlining.IsReadonlyCompoundAssignmentTarget.ConversionFlags.All = 0xffffffnow also switches onUsePrivateProtectedAccessibility/SupportExtensionDeclarationsfor tooltips and compare - probably desirable, but worth calling out in the PR description.CSharpAmbienceprintsoperator +=for a non-public compound operator whileTypeSystemAstBuilderwrites it as a method.CorrectnessTestRunner.roslyn5OrNewerOptionsomitsexecutesCompiledOutput: true.- CallBuilder.cs:1703: "modelled" -> "modeled" (en-US rule in CLAUDE.md).
| if (CSharp.ExpressionBuilder.GetAssignmentOperatorTypeFromMetadataName(operatorCall.Method.Name, context.Settings) == null) | ||
| return false; | ||
| rhs = operatorCall.Arguments[1]; | ||
| valueType = operatorCall.GetParameter(1).Type; |
There was a problem hiding this comment.
operatorCall.GetParameter(1).Type is a ByReferenceType when the static operator takes in Foo / ref readonly Foo. IsShadowedByInstanceOperator wraps that in a plain ResolveResult, and OverloadResolution.CheckApplicability (OverloadResolution.cs:702-728) only strips the parameter's by-ref and then asks for ImplicitConversion(ByReferenceType(Foo) -> Foo), which is None - so no candidate is ever applicable and the shadow check silently returns false.
Repro: a type with static Foo operator +(Foo a, in Foo b) and public void operator +=(in Foo b) (or +=(Foo)); source x = x + y compiles to stloc x(call op_Addition(ldloc x, ldloca y)), this transform emits x += y, and C# 14 binds that to the instance operator (instance phase first) - recompiled code calls a different method.
PrettifyAssignments uses binary.Right.GetResolveResult().Type (by-value) and is fine. Fix here: GetParameter(1).Type.UnwrapByRef() (TypeSystemExtensions.cs:459, as CallBuilder.cs:1752 already does). None of the new fixtures has an in-parameter operator; worth adding one next to BothOperators.
| { | ||
| ResolveResult[] arguments = called.Parameters.Count == 0 | ||
| ? [] | ||
| : [new ResolveResult(called.Parameters[0].Type)]; |
There was a problem hiding this comment.
Same by-ref issue as in HandleCompoundAssign: for operator +=(in T) called.Parameters[0].Type is a ByReferenceType, so no candidate is applicable, PruneCandidatesHiddenByDerivedApplicable prunes nothing, and rebinding through a new operator on a derived receiver type is never detected from ILInlining.CanReplaceCompoundAssignmentReceiver / CopyPropagation.
class Base { public void operator +=(in Foo f) }, class Derived : Base { public new void operator +=(in Foo f) }, source Base b = derived; b += f; -> inlining substitutes ldloc derived for the receiver slot, ReplaceMethodCallsWithOperators then re-checks with the real argument resolve result (231-237), sees the new operator and keeps the call -> derived.op_AdditionAssignment(in f) (not valid C#) instead of the foldable b += f.
called.Parameters[0].Type.UnwrapByRef() (a by-value ResolveResult is the right model; a ByReferenceResolveResult(In) would wrongly skip by-value siblings, see OverloadResolution.cs:686-689). Only the ReplaceMethodCallsWithOperators overload is exercised by the CallInOverload ILPretty case.
| { | ||
| firstArgumentInstruction = new LdObjIfRef(firstArgumentInstruction, typeOfThis); | ||
| } | ||
| else if (materializeReceiver) |
There was a problem hiding this comment.
Evaluation-order bug for the zero-parameter operators (op_IncrementAssignment / op_DecrementAssignment and checked forms): AllocateStackSlot appends stloc S(receiver) to the current block, but the FlushExpressionStack() at 1844-1847 is inside the per-parameter loop, which has zero iterations here - so the receiver read is hoisted above pending side effects on the expression stack.
Reproduced with this branch's ilspycmd on hand-assembled IL ldarg.0; call object Test::A(); ldarg.0; ldfld Counter Test::x; callvirt void Counter::op_IncrementAssignment(); ldarg.0; ldfld x; call Foo(object, Counter) where A() reassigns this.x: output is Counter counter = x; object o = A(); counter++; Foo(o, x); - increments the old x, the original increments the new one. The same IL with a 1-arg op_AdditionAssignment decompiles correctly (object o = A(); x += 1; Foo(o, x);).
Roslyn emits dup for Foo(A(), ++x) so C#-compiled input happens to be unaffected, but any other compiler/weaver/hand IL is not. Fix: if (materializeReceiver) FlushExpressionStack(); before the loop, independent of Parameters.Count.
| // Only an object reference is worth materializing: a value-type receiver is passed by | ||
| // address, so it already denotes a variable, and copying it into another one would | ||
| // make the operator mutate the copy. | ||
| bool materializeReceiver = IsNonStaticOperatorCall() && expectedStackType == StackType.O; |
There was a problem hiding this comment.
materializeReceiver keys on IsNonStaticOperatorCall() (IsOperator && !IsStatic) and is not gated by any setting; UserDefinedCompoundAssign.IsCompoundAssignmentReceiverUse (CompoundAssignmentInstruction.cs:357) uses the same predicate. Neither checks the compound-assignment name or the void return, so every value-returning instance operator - C++/CLI R^ operator+(R^), still SymbolKind.Operator via MetadataMethod.cs:84-90 exactly as before this PR - gets its receiver forced into a stack slot that ILInlining.CanReplaceCompoundAssignmentReceiver (351-361) then refuses to inline unless it is LdLoc/LdObj/LdFlda/LdsFlda.
Net effect on a C++/CLI assembly: GetR().op_Addition(x) / r.Prop.op_Addition(x) / new R().op_Addition(x) regress to R r = GetR(); r.op_Addition(x);, and foreach/using receivers get forced local copies (StatementBuilder 1112-1118, PatternStatementTransform 326-332/674-676, UsingTransform 193-205, CopyPropagation 163-176) - although ReplaceMethodCallsWithOperators will never emit an op= form for a non-*Assignment name. The InstanceOperatorCall ILPretty fixture only has ldarg.0 receivers, so it cannot catch this.
Suggest restricting both predicates to the C# 14 shape (OperatorDeclaration.IsCompoundAssignment(GetOperatorType(name)) + void return) and gating the reader on the setting like every other site.
| /// </summary> | ||
| static IType GetIncrementTargetType(Call call) | ||
| { | ||
| if (call.SlotInfo == StLoc.ValueSlot && call.Parent!.SlotInfo == Block.InstructionSlot) |
There was a problem hiding this comment.
GetIncrementTargetType returns ((StLoc)call.Parent).Variable.Type; for an ILReader StackSlot that is still the System.Object placeholder, so IsShadowedByInstanceOperator finds no candidates, the call is rewritten to stloc S(ldloc x); ++S (86-89), and ExpressionBuilder.VisitStLoc (872-880, HasDefaultStackSlotType true for object) later retypes S to x's real type.
Scenario: Both declares static Both operator ++(Both) and void operator ++(). Foo(++x) reads as stloc S(call op_Increment(ldloc x)); stloc x(ldloc S); call Foo(ldloc S) (S has two loads, not inlined). With MakeAssignmentExpressions=false (TransformAssignment.cs:46-54 skips both inline-assignment transforms) or when TransformInlineAssignmentStObjOrCall bails (impure/used-within target, parameterized setter, 139-199), the object-typed block-level stloc reaches this transform, passes the shadow check against object, and the output Both s = x; ++s; x = s; Foo(s); binds the instance operator ++() under C# 14 instead of the static op_Increment the IL called.
For a StackSlot store variable use call.Arguments[0].InferType(...) / chase the alias like ILInlining.GetReceiverType (ILInlining.cs:375-385), or fall back to call.GetParameter(0).Type.
| // as "x op= y" or "x++". C# resolves that form in two phases, the instance operators | ||
| // on the static type of x first and the static operators only if none of them is | ||
| // applicable, so these candidates are the whole of what recompilation considers here. | ||
| // The fallback phase is not modelled: a call this set cannot account for gives up the |
There was a problem hiding this comment.
nit: "modelled" -> "modeled" (en-US per CLAUDE.md).
| } | ||
| // IsCompoundStore accepts a store to a local (StLoc) or to a field, array element, | ||
| // ref or pointer (StObj), which are variables, and a setter call, which is not. | ||
| if (CSharpResolver.IsShadowedByInstanceOperator(operatorCall.Method, targetType, valueType, |
There was a problem hiding this comment.
Cost/gating: IsShadowedByInstanceOperator is called from four sites (here, 906, FixRemainingIncrements.cs:51, PrettifyAssignments.cs:112) without a Settings.UserDefinedCompoundAssignmentOperators gate, and each call walks the full type hierarchy twice (plain name + checked name via two GetInstanceOperatorCandidates calls, CSharpResolver.cs:1304-1306) plus an O(n^2) GetBaseMembers dedup (1266-1270).
So every x = x + y / ++x on any user-defined type (decimal, DateTime, TimeSpan, BigInteger, ...) reaching these transforms now pays two BaseTypeCollector walks over every method of every non-interface base type - even when the setting is off, in which case DecompilerTypeSystem.GetOptions never classifies op_*Assignment as Operator and a candidate can never be found. ILInlining / CopyPropagation / StatementBuilder / UsingTransform all gate on the setting; these four do not.
Cheaper: gate on the setting (parameter or call site), collect both names in one GetMethods pass (m.Name == name || m.Name == checkedName), and if (candidates.Count <= 1) return candidates; before the dedup.
| return false; | ||
| foreach (var load in target.LoadInstructions) | ||
| { | ||
| if (load.Parent is not CallInstruction { Method: { IsOperator: true, IsStatic: false } } call |
There was a problem hiding this comment.
This re-inlines the Parent is CallInstruction { Method: { IsOperator: true, IsStatic: false } } && Arguments[0] == load match instead of calling UserDefinedCompoundAssign.IsCompoundAssignmentReceiverUse, which this PR adds and which ILInlining.cs:269 and UsingTransform.cs:195/197 already use. Tightening the shared helper later (e.g. to the compound-assignment/void shape, or adding the Accessibility == Public requirement ReplaceMethodCallsWithOperators.cs:199 has) would leave CopyPropagation on the old rule. if (!UserDefinedCompoundAssign.IsCompoundAssignmentReceiverUse(load)) continue; var call = (CallInstruction)load.Parent!;
| /// "static member (+=)" to a static, value-returning op_AdditionAssignment, and C++/CLI emits | ||
| /// value-returning instance operators. Those are plain methods as far as C# is concerned. | ||
| /// </summary> | ||
| /// <summary> |
There was a problem hiding this comment.
Two stacked <summary> blocks: the first one (shape: instance, void, one/zero parameters, F# / C++/CLI note) describes IsCompoundAssignmentOperatorSignature, which currently has no doc comment; move it up there.
| /// Gets whether the operator type is a C# 14 user-defined compound assignment operator | ||
| /// (a void-returning instance operator, including the increment/decrement forms). | ||
| /// </summary> | ||
| public static bool IsCompoundAssignment(OperatorType type) |
There was a problem hiding this comment.
type >= AdditionAssignment depends on the enum order staying as it is; an explicit switch (like IsChecked next to it) or deriving from the names table would not break silently when someone appends a non-assignment member to OperatorType.
Implements decompiler support for C# 14 user-defined compound assignment operators (#829): instance operator declarations decompile to
public void operator +=(T rhs)/operator checked +=/operator ++(), and call sites fold back tox += y;/x++;. Cross-checked against Roslyn 5.9 throughout.The branch is three commits, one per subsystem.
Recognize (type system)
C# 14 emits an instance compound assignment operator as a void-returning
op_*Assignmentmethod.MetadataMethodclassifies such a method as an operator only when it has the shape C# requires — an instance, void-returning method with the right arity (one parameter, none for++/--) and noref/paramsparameter. F# manglesstatic member (+=)to a static, value-returningop_AdditionAssignmentand C++/CLI emits value-returning instance operators; both stay plain methods. Explicit interface implementations, whose metadata carries only the dotted name and nospecialname, are recognized too, sovoid ICompound<int>.operator +=(int rhs)round-trips.Recognition is a C# 14 feature, so it is gated by a
UserDefinedCompoundAssignmentOperatorsdecompiler setting and a matchingTypeSystemOptionsflag (the way the extension-method classification already is). Below C# 14 the methods stay plainop_*Assignmentmethods and keep theirspecialnameflag as a[SpecialName]attribute;[IsReadOnly]is now surfaced for operators so areadonlystruct operator prints with the modifier. TheCompilerFeatureRequiredattribute Roslyn stamps on each operator is removed when operator syntax is used.The resolver gains the two-phase rules
x op= ybinds by, next toGetUserDefinedOperatorCandidates: the instance operators reachable from the static type of x are considered first, the static operators only when none is applicable. These candidate/applicability/shadowing helpers live onCSharpResolverrather than on an IL instruction — they are overload-resolution logic that never touches an IL node.Decompile (transforms)
A call to an instance compound assignment operator,
X::op_AdditionAssignment(x, y), folds back intox += y(andx++, the checked forms), rewritten at the AST level inReplaceMethodCallsWithOperatorsrather than through a new IL instruction. The form takes its operator from the static type ofxand needsxto stay an assignable variable, so the receiver is protected end to end:base,thisin a class,inparameters, readonly fields, and receivers the compiler optimized away keep the explicit call. Inlining and copy propagation share one predicate that refuses to replace the receiver with a non-lvalue, or with a value whose static type would bind a different operator (a base-class or interface operator).foreachandusingstill emit their statement, redirecting the operator's receiver to a fresh copy where the loop/using variable would otherwise be read-only.static C operator +(C, int)andvoid operator +=(int),x = x + ystays spelled out: under C# 14x += ywould bind the instance operator and mutate in place. The guard consults the same two-phase candidate set as everything else, so it foldsarr[0] = arr[0] + 1only when no instance operator shadows the static one — the case that previously rebound silently, printing1before decompilation and100after.>>>=names followCheckedOperators/UnsignedRightShiftlike every other operator name, at call sites and declarations alike.Render (UI)
Instance compound assignment operators show as
operator +=(int) : voidin the tree and tooltips instead of theirop_*Assignmentmetadata name; ilspycmd's-lvhelp lists the C# 14 and 15 language versions.Not covered
Result-used forms like
d = (c += 5)decompile as two equivalent statements; and a Release build where the compiler erased a local holding a receiver whose type carries anew-shadowed operator keeps the explicit call — correct but not recompilable, since re-introducing the temporary would have to happen at the ILAst level.Tests
The
UserDefinedCompoundAssignmentpretty fixture covers all 19 operators, the call-site shapes above, inheritance, explicit interface implementation, and a type declaring both the static and the instance operator.UserDefinedCompoundAssignmentInheritanceruns the program before and after decompilation for every combination of where a staticoperator +and an instanceoperator +=are declared across two levels — the only test kind that catches a silent rebinding, since the decompiled text looks fine.CompoundAssignmentOperatorEdgeCasespins the hand-written-IL cases that have no C# spelling: the F#/C++ method shapes, abasecall, a non-public operator, a mismatched receiver type, an unconstrained generic receiver, and a shadowed static increment.NoUserDefinedCompoundAssignmentOperatorspins the output with the setting turned off. Full ICSharpCode.Decompiler.Tests sweep: 3467 total, 0 failed, 46 skipped.🤖 Generated with Claude Code